Skip to content

feat: add MCode as a built-in ACP agent - #503

Open
hetaoBackend wants to merge 3 commits into
openclaw:mainfrom
hetaoBackend:feat/mcode-agent
Open

feat: add MCode as a built-in ACP agent#503
hetaoBackend wants to merge 3 commits into
openclaw:mainfrom
hetaoBackend:feat/mcode-agent

Conversation

@hetaoBackend

@hetaoBackend hetaoBackend commented Aug 16, 2026

Copy link
Copy Markdown

Closes #502

What Problem This Solves

MiniMax Code already exposes a native ACP v1 server through mcode acp, but acpx users currently have to configure that command manually. This makes MCode unavailable through the same built-in agent workflow as Codex, Claude, Gemini, and the other supported ACP agents.

Why This Change Was Made

Add mcode to both built-in command registries as structured argv (["mcode", "acp"]), with registry and cross-platform CLI integration coverage. The agent guide documents installation, authentication, permission handling, and the current session/load limitation without adding an adapter or sidecar.

User Impact

After installing and authenticating MiniMax Code, users can run:

acpx mcode exec 'summarize this repository'

MCode remains responsible for its own runtime, login state, model configuration, and tools; acpx only owns the ACP client lifecycle and permission policy.

Evidence

  • pnpm run check — 918 tests passed; coverage thresholds passed (94.73% lines, 87.94% branches)
  • pnpm run check:docs — formatting, markdown lint, and docs site build passed
  • Focused registry test passed after first demonstrating the missing built-in as a failing test
  • Focused CLI integration test verifies mcode acp resolution on Unix and Windows .cmd launch paths
  • Live smoke with @minimax-ai/code 0.1.2 returned ACPX_MCODE_OK through acpx mcode exec; the redacted transcript is below
  • Live JSON smoke completed ACP initialize/session/prompt, advertised loadSession: false, streamed a terminal tool-call lifecycle, and returned the expected result; the redacted protocol excerpt is below
  • Project autoreview: clean, no accepted/actionable findings

Redacted real-agent transcript

This was rerun from the PR checkout against the installed, authenticated
@minimax-ai/code 0.1.2 binary. The working-directory path and per-session/message/tool
identifiers are omitted; no protocol result fields were changed.

$ mcode --version
0.1.2

$ ./node_modules/.bin/tsx src/cli.ts --format quiet --timeout 180 --deny-all --cwd <temporary-directory> mcode exec 'Reply with exactly ACPX_MCODE_OK and do not use tools.'
ACPX_MCODE_OK

The second run used --format json --json-strict --approve-all, asked MCode to execute
printf ACPX_TOOL_OK, and filtered the transcript to the initialize response, tool
lifecycle, final agent message, and prompt result:

{"jsonrpc":"2.0","id":0,"result":{"protocolVersion":1,"agentCapabilities":{"loadSession":false,"mcpCapabilities":{"http":true,"sse":true},"promptCapabilities":{"image":false,"audio":false,"embeddedContext":false}},"agentInfo":{"name":"minimax-code","title":"MiniMax Code","version":"0.1.2"}}}
{"jsonrpc":"2.0","method":"session/update","params":{"update":{"sessionUpdate":"tool_call","toolCallId":"<redacted>","title":"bash","name":"bash","kind":"execute","status":"in_progress","rawInput":{"command":"printf ACPX_TOOL_OK"}}}}
{"jsonrpc":"2.0","method":"session/update","params":{"update":{"sessionUpdate":"tool_call_update","toolCallId":"<redacted>","status":"completed","rawInput":{"command":"printf ACPX_TOOL_OK"},"rawOutput":{"content":[{"type":"text","text":"ACPX_TOOL_OK"}]}}}}
{"jsonrpc":"2.0","method":"session/update","params":{"update":{"sessionUpdate":"agent_message_chunk","messageId":"<redacted>","content":{"type":"text","text":"ACPX_TOOL_OK"}}}}
{"jsonrpc":"2.0","id":2,"result":{"stopReason":"end_turn"}}

Scope Notes

  • No MCode protocol changes are included.
  • MCode 0.1.2 does not advertise provider-session reload. The docs now show only one-shot exec usage and explicitly state that sequential CLI invocations start fresh MCode context.

AI assistance was used to investigate the integration path, implement the change, run validation, and draft this PR. All generated changes and evidence were reviewed before submission.

@clawsweeper

clawsweeper Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@hetaoBackend
hetaoBackend marked this pull request as ready for review August 16, 2026 13:15
@hetaoBackend
hetaoBackend requested a review from a team as a code owner August 16, 2026 13:15
@clawsweeper clawsweeper Bot added merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Aug 16, 2026
@clawsweeper

clawsweeper Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Codex review: found issues before merge. Reviewed August 24, 2026, 6:07 PM ET / 22:07 UTC.

ClawSweeper review

What this changes

The branch registers MiniMax Code’s mcode acp server as a built-in agent, adds its documentation, and tests command resolution on Unix and Windows.

Merge readiness

⚠️ Needs maintainer review before merge - 4 items remain

Keep open for an explicit product decision. The branch has credible live ACP evidence and current main does not include MCode, but reserving mcode turns an already-supported configurable command into a permanent built-in contract.

Priority: P3
Reviewed head: 5b40647f7cf48e7f747191afeacc9110ef5c3c36
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The focused implementation and live proof are solid, but the unapproved permanent keyword contract remains a merge blocker.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (live_output): The PR body provides redacted after-fix live output from an authenticated MCode 0.1.2 run, including ACP initialization, a completed tool lifecycle, and the expected final result.
Patch quality 🦐 gold shrimp (3/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Verified Sufficient (live_output): The PR body provides redacted after-fix live output from an authenticated MCode 0.1.2 run, including ACP initialization, a completed tool lifecycle, and the expected final result.
Evidence reviewed 6 items Current main does not contain the proposed built-in: The current-main registry has no mcode entry; the branch adds the string and structured argv entries.
Existing supported configuration path: Invocation resolution chooses a configured agent before built-in argv/command resolution, and the custom-agent documentation states that configuration can define and override friendly names.
Keyword policy: VISION.md defines keywords as API surface and says the default stance is to add fewer stable conventions.
Findings 1 actionable finding [P1] Obtain approval before reserving mcode
Security None None.

How this fits together

ACPX resolves an agent request by honoring an explicit configured command before falling back to its built-in registry. The resolved structured command starts an ACP server and returns its messages and permission requests to the CLI.

flowchart LR
  A[CLI agent request] --> B[Configured command lookup]
  B -->|Configured| C[User agent command]
  B -->|Not configured| D[Built-in agent registry]
  C --> E[ACP process launch]
  D --> E
  E --> F[MCode ACP server]
  F --> G[Agent messages and permission requests]
Loading

Decision needed

Question Recommendation
Should ACPX reserve mcode as a stable supported built-in keyword instead of leaving MCode as an explicit custom-agent mapping? Keep MCode configurable: Retain the existing custom-agent route unless an owner chooses to make MCode a permanent supported registry contract.

Why: The implementation is narrow, but ACPX policy treats keywords as durable product API and current configuration already provides the underlying capability.

Before merge

  • Obtain approval before reserving `mcode` (P1) - Configured agents already take precedence over built-ins, so users can map this command today. This new registry row changes that opt-in setup into a durable supported keyword; obtain the required owner decision or keep the existing custom-agent path.
  • Resolve merge risk (P1) - Merging permanently reserves mcode and makes a currently opt-in configuration convention a supported default keyword without an owner-approved support commitment.
  • Complete next step (P2) - A maintainer must decide whether the new permanent keyword is a supported product contract; this is not a mechanical repair-lane task.

Findings

  • [P1] Obtain approval before reserving mcodesrc/agent-registry.ts:54
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Diff composition production +2, tests +67, docs +65 The implementation is a small registry addition with substantial command-resolution coverage and synchronized harness documentation.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #502
Summary: This PR is the explicit candidate implementation for the linked open product proposal.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

  1. Keep MCode configurable (recommended)
    Do not reserve the keyword until an owner elects to carry its ongoing compatibility and support contract.
  2. Approve the registry contract
    Explicitly accept mcode as a stable built-in and proceed with the branch after a current-main refresh.

Technical review

Best possible solution:

Have the registry owner explicitly accept or decline mcode as a stable built-in; if accepted, retain the documented exec-first limitation and refresh the branch before merge, otherwise keep the existing custom-agent path.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this PR proposes a new built-in keyword rather than reporting broken existing behavior; its body nevertheless includes a redacted real-agent ACP run.

Is this the best way to solve the issue?

Unclear: the registry row is mechanically narrow, but current configuration already provides the command and only an owner can decide whether its convenience merits a permanent keyword contract.

Full review comments:

  • [P1] Obtain approval before reserving mcodesrc/agent-registry.ts:54
    Configured agents already take precedence over built-ins, so users can map this command today. This new registry row changes that opt-in setup into a durable supported keyword; obtain the required owner decision or keep the existing custom-agent path.
    Confidence: 0.96

Overall correctness: patch is incorrect
Overall confidence: 0.94

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against d4c16ab32154.

Labels

Label justifications:

  • P3: This is a non-urgent built-in integration proposal awaiting a product decision.
  • merge-risk: 🚨 compatibility: The new default keyword creates a durable support and compatibility promise beyond the existing opt-in configuration path.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR body provides redacted after-fix live output from an authenticated MCode 0.1.2 run, including ACP initialization, a completed tool lifecycle, and the expected final result.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides redacted after-fix live output from an authenticated MCode 0.1.2 run, including ACP initialization, a completed tool lifecycle, and the expected final result.

Evidence

What I checked:

  • Current main does not contain the proposed built-in: The current-main registry has no mcode entry; the branch adds the string and structured argv entries. (src/agent-registry.ts:54, d4c16ab32154)
  • Existing supported configuration path: Invocation resolution chooses a configured agent before built-in argv/command resolution, and the custom-agent documentation states that configuration can define and override friendly names. (src/cli/flags.ts:560, 2d735cf18220)
  • Keyword policy: VISION.md defines keywords as API surface and says the default stance is to add fewer stable conventions. (VISION.md:79, d4c16ab32154)
  • MCode protocol dependency is explicit: The changed agent guide declares that the registered command starts MCode’s native ACP v1 stdio server; the PR body also includes a redacted authenticated ACP v1 transcript. (agents/MCode.md:7, 5b40647f7cf4)
  • Current-main ownership signal: Current invocation routing and registry merge behavior blame to the v0.13.1 release commit authored by Peter Steinberger. (src/agent-registry.ts:128, 2d735cf18220)
  • Not shipped or implemented: No release tag contains the PR head, and the latest release v0.13.1 is based on a different commit; this remains an open feature proposal rather than an implemented fix. (5b40647f7cf4)

Likely related people:

  • steipete: Current-main blame attributes the central configured-before-built-in resolution and registry merge path to Peter Steinberger, and the contributor explicitly requested this product decision from steipete. (role: recent registry and invocation-routing contributor; likely product decision owner; confidence: medium; commits: 2d735cf18220; files: src/cli/flags.ts, src/agent-registry.ts)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Obtain an explicit decision from the registry owner on whether mcode should be a supported built-in keyword.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (5 earlier review cycles)
  • reviewed 2026-08-16T13:21:18.339Z sha 79b5bf4 :: needs real behavior proof before merge. :: [P2] Remove the contributor-owned changelog entry
  • reviewed 2026-08-16T14:28:49.624Z sha ddafca4 :: needs changes before merge. :: [P2] Remove the persistent-session example
  • reviewed 2026-08-16T14:34:28.761Z sha 5b40647 :: found issues before merge. :: [P1] Avoid reserving mcode without an approved registry need
  • reviewed 2026-08-21T22:22:09.715Z sha 5b40647 :: found issues before merge. :: [P1] Avoid reserving mcode without an approved registry need
  • reviewed 2026-08-22T03:55:48.190Z sha 5b40647 :: found issues before merge. :: [P1] Avoid reserving mcode without an approved registry need

@hetaoBackend

Copy link
Copy Markdown
Author

Addressed the actionable review items in ddafca4:

  • Removed the contributor-owned CHANGELOG.md entry; release notes remain owned by the release process.
  • Reran the integration against the installed, authenticated @minimax-ai/code 0.1.2 binary.
  • Added a redacted, inspectable transcript to the PR body covering ACP v1 initialization, the advertised loadSession: false boundary, a completed terminal tool call, the final agent message, and end_turn.
  • Reran pnpm run check (918/918 tests, coverage gates passed) and pnpm run check:docs successfully on the updated checkout.

The remaining question—whether mcode should be accepted as a permanent built-in keyword—is intentionally left to maintainer product judgment.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Aug 16, 2026
@hetaoBackend

Copy link
Copy Markdown
Author

Addressed the remaining author-side documentation finding in 5b40647:

  • Removed the sessions new plus later bare-prompt example from agents/MCode.md.
  • Documented that sessions new closes the ACP client after saving the local record and that, without provider-session reload, a later CLI invocation starts fresh MCode context.
  • Synchronized the same exec-first boundary in docs/agents.md.
  • Verified pnpm run check:docs and explicit Markdown lint for agents/README.md and agents/MCode.md.

The real-agent proof remains in the PR body. The only remaining question is maintainer approval of mcode as a permanent built-in keyword.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@hetaoBackend

Copy link
Copy Markdown
Author

@steipete Would you mind taking a look and making the product call on this one?

I’d like to get MCode included as a built-in ACP agent. While mcode acp can already be configured through the custom-agent path, a built-in entry gives users the same straightforward discovery and invocation experience as the other supported agents:

acpx mcode exec 'summarize this repository'

The implementation is intentionally narrow: it only registers the native mcode acp argv, without an adapter or sidecar. Cross-platform registry/CLI coverage, the full test suite, docs checks, and authenticated real-agent ACP smoke tests all pass. The current lack of provider-session reload is also documented explicitly.

ClawSweeper’s remaining concern is the long-term keyword/support contract rather than an implementation defect. I’m in favor of accepting that contract and merging the PR, but I’d appreciate your decision as the maintainer familiar with this registry boundary. If you’re open to the built-in, I’m happy to address any final changes you’d like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: add MCode as a built-in ACP agent

1 participant